Skip to content

feat: rework PlanDisplay to Plans 2.0 design - #170

Open
noecondoleo wants to merge 9 commits into
masterfrom
feat/plan-display-plans-2
Open

feat: rework PlanDisplay to Plans 2.0 design#170
noecondoleo wants to merge 9 commits into
masterfrom
feat/plan-display-plans-2

Conversation

@noecondoleo

@noecondoleo noecondoleo commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Make the Plans 2.0 layout the canonical PlanDisplay, matching the Figma master component:

  • Card: default level-2 shadow, plus opt-in ribbonLabel and gradient variants; remove the legacy highlighted prop (breaking change)
  • Add PlanDisplay.Price subcomponent (current/previous price, period, annual note)
  • Add tag slot on Header, badge slot on Bullet, icon slot on Footer
  • Update stories, README, CHANGELOG and version bump

Type

  • Bugfix 🐛
  • New feature 🌈
  • Change request 🤓
  • Documentation 📚
  • Tech debt 👩‍💻

Changes proposed ✔️

Summary by CodeRabbit

Release Notes

  • New Features

    • Reworked PlanDisplay component with Plans 2.0 canonical layout
    • Added new component slots: tag, badge, and icon support
    • Introduced PlanDisplay.Price subcomponent
    • New card visual variants: ribbonLabel and gradient
  • Breaking Changes

    • Removed highlighted prop; use ribbonLabel or gradient instead
  • Documentation

    • Updated README with subcomponents guide and usage examples
    • Added Plans 2.0 example section
    • Enhanced Storybook with interactive playground

Make the Plans 2.0 layout the canonical PlanDisplay, matching the Figma
master component:

- Card: default level-2 shadow, plus opt-in `ribbonLabel` and `gradient`
  variants; remove the legacy `highlighted` prop (breaking change)
- Add `PlanDisplay.Price` subcomponent (current/previous price, period,
  annual note)
- Add `tag` slot on Header, `badge` slot on Bullet, `icon` slot on Footer
- Update stories, README, CHANGELOG and version bump

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 3059e4eb-9803-4bf2-963f-0293892de8dd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key: "auto_resolve_threads"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
📝 Walkthrough

Walkthrough

PlanDisplay is redesigned for Plans 2.0. A new PlanDisplayCardPrice subcomponent is introduced and exposed as PlanDisplay.Price. PlanDisplayCard replaces the highlighted boolean with ribbonLabel and gradient props. PlanDisplayCardHeader, PlanDisplayBullet, and PlanDisplayCardFooter gain tag, badge, and icon slots, respectively. PlanDisplayCardSpacing switches from border styling to a Divider. Stories, tests, README, and CHANGELOG are updated throughout.

Changes

Plans 2.0 PlanDisplay Overhaul

Layer / File(s) Summary
New PlanDisplayCardPrice subcomponent
src/components/PlanDisplayCardPrice/PlanDisplayCardPrice.tsx, src/components/PlanDisplayCardPrice/planDisplayCardPrice.types.ts, src/components/PlanDisplayCardPrice/index.ts, src/components/PlanDisplayCardPrice/planDisplayCardPrice.spec.tsx, src/components/PlanDisplayCardPrice/planDisplayCardPrice.stories.tsx
Defines PlanDisplayCardPriceProperties (price, previousPrice, period, annualNote all typed as ReactNode), implements the rendering layout (price as Title heading, line-through previous price, inline period, caption annual note), and ships unit tests and three Storybook story variants.
PlanDisplayCard ribbonLabel/gradient refactor
src/components/PlanDisplayCard/planDisplayCard.types.ts, src/components/PlanDisplayCard/PlanDisplayCard.tsx, src/components/PlanDisplayCard/planDisplayCard.spec.tsx
Removes highlighted from PlanDisplayCardProperties and adds gradient?: boolean and ribbonLabel?: ReactNode. The component now conditionally renders a ribbon header section or selects a gradient/neutral background. Tests are updated to cover both new variants.
PlanDisplayCardHeader tag slot
src/components/PlanDisplayCardHeader/planDisplayCardHeader.types.ts, src/components/PlanDisplayCardHeader/PlanDisplayCardHeader.tsx, src/components/PlanDisplayCardHeader/planDisplayCardHeader.spec.tsx
Adds tag?: ReactNode to PlanDisplayCardHeaderProperties. When present, subtitle and tag are rendered side-by-side in a space-between flex row; when absent, subtitle renders alone with neutral-textHigh color. A new test asserts tag slot presence.
PlanDisplayBullet badge slot and style updates
src/components/PlanDisplayBullet/planDisplayBullet.types.ts, src/components/PlanDisplayBullet/PlanDisplayBullet.tsx, src/components/PlanDisplayBullet/planDisplayCardBullet.spec.tsx
Adds badge?: ReactNode to PlanDisplayBulletProperties. When provided, renders children followed by an inline-flex badge wrapper. Icon color changes to primary-interactive and text fontWeight changes to regular.
Footer icon slot, Spacing Divider, Content padding
src/components/PlanDisplayCardFooter/..., src/components/PlanDisplayCardSpacing/PlanDisplayCardSpacing.tsx, src/components/PlanDisplayCardContent/PlanDisplayCardContent.tsx
Adds icon?: ReactNode to PlanDisplayCardFooterProperties; footer now always renders a Divider and conditionally renders icon+children in a flex row. PlanDisplayCardSpacing replaces border-styled Box with a Divider component. PlanDisplayCardContent padding changes from py="4" to pb="4".
PlanDisplay.Price wiring and barrel exports
src/PlanDisplay.tsx, src/planDisplay.types.ts, src/components/index.ts
Imports PlanDisplayCardPrice, assigns it to PlanDisplay.Price with displayName, adds Price: typeof PlanDisplayCardPrice to the PlanDisplayComponents interface, and adds a barrel re-export.
Stories, README, and CHANGELOG
src/planDisplay.stories.tsx, README.md, CHANGELOG.md
Replaces twoPlans/threePlans stories with Default and Playground stories covering all new slots and variants. Adds a Subcomponents reference section and Plans 2.0 TSX composition example to README. Adds a 2.0.0 CHANGELOG entry documenting features and the highlighted breaking change.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

enhancement


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Breaking Change Documentation Verification ❌ Error CHANGELOG.md has breaking changes section with "⚠️ Breaking changes" header and documents removal of highlighted prop, but MISSING required PR reference (#PR format); only author attributi... Add PR reference to breaking change entry: append "(#PR" after description and before author attribution to match format: "- Description. (#PR by @author)"
Title check ⚠️ Warning The title uses 'feat:' prefix and describes the main change (rework PlanDisplay to Plans 2.0 design), but does not follow the required format of 'chore/fix/feat(PackageName): Title'. Update the title to 'feat(PlanDisplay): rework to Plans 2.0 design' to match the required format with package name in parentheses.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Unversioned Breaking Changes ✅ Passed Breaking change (removed highlighted prop from PlanDisplayCard) is properly versioned with MAJOR bump to 2.0.0 in CHANGELOG, meeting semantic versioning requirements.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/plan-display-plans-2

Comment @coderabbitai help to get the list of available commands.

@snyk-io-us

snyk-io-us Bot commented Jun 23, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions

Copy link
Copy Markdown
Contributor

Getting started

Please make sure you read our documentation on how to write code for components, stories and styles.

  • Provide a list of changes
  • Include images to help better visualize your work
  • If your PR closes an issue, please link it
  • Add reviewers
  • Add a label to help better understand what your changes are related to

@coderabbitai coderabbitai Bot added the enhancement New feature or request label Jun 23, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 15

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/react/src/components/PlanDisplay/CHANGELOG.md`:
- Around line 7-13: The CHANGELOG.md file uses non-standard headers and
incomplete PR references that don't match the repository contract. Update the
category headers from "### ✨ Features" to "### 🎉 New features" and from "### ⚠️
Breaking changes" to "### 🛠 Breaking changes". Ensure each changelog entry
follows the required format by including the complete PR reference as
`([`#PR`](full-url) by [`@contributor`](profile-url))` at the end of each item
description. The entries mentioning the PlanDisplay rework and the highlighted
prop removal need to have their PR references properly formatted with the full
GitHub URL and contributor profile URL.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayBullet/PlanDisplayBullet.tsx`:
- Around line 28-33: Remove the inline style prop containing verticalAlign:
"middle" from the Box component wrapper in the PlanDisplayBullet component.
Replace it with design-system-supported alignment props instead. Since the
intent is vertical alignment, use the appropriate Box layout prop (such as
alignItems) that matches the design system's conventions for handling vertical
centering, removing the custom inline style entirely.
- Around line 14-19: The Box component is nested inside the Text component that
renders as a span (inline element), which creates invalid HTML when Box renders
as a block element. To fix this, modify the Box component to render as a span by
adding the as="span" prop, or if the Box wrapper is not necessary for layout
purposes, remove it entirely and place the icon directly inside the Text
component with appropriate flex styling applied to the Text element instead.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayBullet/planDisplayBullet.types.ts`:
- Around line 3-10: Convert the PlanDisplayBulletProperties type alias to an
exported interface. Replace the type keyword and assignment syntax with
interface syntax, keeping all the properties (icon, disabled, badge) and their
JSDoc comments intact. This aligns with the repository's coding guidelines that
require ComponentNameProperties to be exported as an interface in *.types.ts
files.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCard/planDisplayCard.spec.tsx`:
- Around line 21-32: Add a new test case in the planDisplayCard.spec.tsx file
that verifies the precedence behavior where ribbonLabel overrides gradient when
both props are provided. Create a test using makeSut that passes both gradient:
true and a ribbonLabel value, then assert that the ribbon label text is
rendered, confirming that the ribbon takes precedence over the gradient variant
as documented in the public contract.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCard/PlanDisplayCard.tsx`:
- Around line 18-29: Replace the native HTML div element with inline styles
(starting at the display: "flex" container) with a Box component from
`@nimbus-ds/components`. Convert all inline CSS properties to Box props using
Nimbus design tokens: transform display/justifyContent/alignItems to their
corresponding Box props, replace the backgroundColor value with a Nimbus color
token, convert paddingTop/paddingBottom to pt and pb props using spacing tokens,
replace borderTopLeftRadius/borderTopRightRadius with borderRadius using shape
tokens, and convert the marginBottom negative calculation to the mb prop. Apply
this same refactoring to the other instance mentioned in lines 52-59 to ensure
consistent use of Nimbus primitives throughout the PlanDisplayCard component.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardFooter/planDisplayCardFooter.types.ts`:
- Around line 3-10: Convert PlanDisplayCardFooterProperties from a type alias to
an interface that contains only the component-specific props (the optional icon
property with its documentation). Remove the PropsWithChildren wrapper from the
interface definition itself. If the component implementation requires
PropsWithChildren composition, create a separate internal type that composes the
interface with PropsWithChildren for use within the component file.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardHeader/planDisplayCardHeader.types.ts`:
- Around line 3-14: The PlanDisplayCardHeaderProperties in the .types.ts file is
currently defined as a type alias but project guidelines require it to be
defined as an interface. Convert the type alias declaration for
PlanDisplayCardHeaderProperties from using the type keyword syntax to an
interface keyword syntax, maintaining the same properties and their
documentation comments. The interface should have the same structure with
subtitle, title, and optional tag properties.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/planDisplayCardPrice.spec.tsx`:
- Around line 19-31: Add a new negative-path test case for the
PlanDisplayCardPrice component that validates optional props are not rendered
when omitted. Create a test that calls makeSut with only required props (price)
and excludes previousPrice, period, and annualNote, then use screen.queryByText
assertions to verify these optional elements are not present in the rendered
output. This complements the existing positive-path test and ensures full
coverage of the component's behavior when optional props are not provided.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/planDisplayCardPrice.stories.tsx`:
- Around line 10-14: The `annualNote` control is explicitly disabled in the
argTypes configuration of the planDisplayCardPrice.stories.tsx file, which
violates the Storybook guideline requiring controls for all component props.
Either remove the `control: { disable: true }` setting from the `annualNote`
argTypes entry to enable the default control, or if the prop is difficult to
edit directly as a ReactNode, create a serializable alternative control such as
`annualNoteText` and use a render function to map this serializable value to the
actual `annualNote` prop passed to the component.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/PlanDisplayCardPrice.tsx`:
- Around line 20-24: Remove the inline style object from the previousPrice span
element in PlanDisplayCardPrice.tsx that contains custom color and
textDecoration properties. Replace this inline styling with appropriate Nimbus
design-system tokens and props. Check if Nimbus provides component props or
token utilities for applying neutral-interactivePressed color and line-through
text decoration, and use those instead of the style={{ }} object.
- Around line 5-44: The PlanDisplayCardPrice component is missing required
documentation and displayName property per the repo's TSX standards. Add a JSDoc
comment block above the component definition that includes a description of the
component's purpose and documents all props (price, previousPrice, period,
annualNote) with their types and descriptions. Additionally, add
PlanDisplayCardPrice.displayName = "PlanDisplayCardPrice" after the component
export to explicitly set the display name for debugging and React DevTools
identification.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/planDisplayCardPrice.types.ts`:
- Around line 3-27: Convert the PlanDisplayCardPriceProperties type alias to an
interface declaration and add a top-level JSDoc comment above it. Change the
`export type PlanDisplayCardPriceProperties = { ... }` syntax to `export
interface PlanDisplayCardPriceProperties { ... }` and add a descriptive JSDoc
comment above the interface that explains what this type represents, keeping the
existing field-level JSDoc comments intact.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardSpacing/PlanDisplayCardSpacing.tsx`:
- Line 8: The Box component in PlanDisplayCardSpacing.tsx currently has the
my="3" margin applied after the {...props} spread, which prevents consumers from
overriding the default margin value. Reorder the attributes so that the default
spacing (my="3") is set before spreading the props, allowing any margin-related
props passed by consumers to override the default. Change the order from
`{...props} my="3"` to `my="3" {...props}`.

In `@packages/react/src/components/PlanDisplay/src/planDisplay.stories.tsx`:
- Around line 155-184: The Playground story object only exposes boolean
visibility toggles through PlaygroundArgs type and argTypes, but does not
include controls for important content props like ribbonLabel, gradient, price,
previousPrice, period, and annualNote. Add these missing props to the
PlaygroundArgs type definition with appropriate types, provide default values
for each in the args object, and define corresponding controls in the argTypes
object using appropriate control types (text for string values, number for
numeric values, etc.) to make all component props discoverable and configurable
in Storybook.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 1c8feacc-938c-443f-ac7f-f0ed7a0f1cdc

📥 Commits

Reviewing files that changed from the base of the PR and between 0876046 and f5bc5bf.

⛔ Files ignored due to path filters (1)
  • .yarn/versions/plan-display-plans-2.yml is excluded by !**/.yarn/**
📒 Files selected for processing (25)
  • packages/react/src/components/PlanDisplay/CHANGELOG.md
  • packages/react/src/components/PlanDisplay/README.md
  • packages/react/src/components/PlanDisplay/src/PlanDisplay.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayBullet/PlanDisplayBullet.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayBullet/planDisplayBullet.types.ts
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayBullet/planDisplayCardBullet.spec.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCard/PlanDisplayCard.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCard/planDisplayCard.spec.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCard/planDisplayCard.types.ts
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardContent/PlanDisplayCardContent.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardFooter/PlanDisplayCardFooter.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardFooter/planDisplayCardFooter.spec.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardFooter/planDisplayCardFooter.types.ts
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardHeader/PlanDisplayCardHeader.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardHeader/planDisplayCardHeader.spec.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardHeader/planDisplayCardHeader.types.ts
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/PlanDisplayCardPrice.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/index.ts
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/planDisplayCardPrice.spec.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/planDisplayCardPrice.stories.tsx
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/planDisplayCardPrice.types.ts
  • packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardSpacing/PlanDisplayCardSpacing.tsx
  • packages/react/src/components/PlanDisplay/src/components/index.ts
  • packages/react/src/components/PlanDisplay/src/planDisplay.stories.tsx
  • packages/react/src/components/PlanDisplay/src/planDisplay.types.ts

Comment on lines +7 to +13
### ✨ Features

- Reworked `PlanDisplay` to the Plans 2.0 design as the canonical layout, matching the Figma master component. `PlanDisplay.Card` now renders the default card with a level-2 shadow, plus two opt-in variants: `ribbonLabel` (a "Más escogido" ribbon with a primary 2px border) and `gradient` (primary-surface → background gradient). Added a `tag` slot on `PlanDisplay.Header` (e.g. "Plan actual"), a new `PlanDisplay.Price` subcomponent (current price + optional previous price, period and annual note), a `badge` slot on `PlanDisplay.Bullet` (e.g. "Nuevo"), and an `icon` slot on `PlanDisplay.Footer` for add-on rows. (by [@noecondoleo](https://github.com/noecondoleo))

### ⚠️ Breaking changes

- Removed the legacy `highlighted` prop on `PlanDisplay.Card`. Use `ribbonLabel` for the featured plan or `gradient` for the gradient variant. (by [@noecondoleo](https://github.com/noecondoleo))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Fix CHANGELOG format to match repository contract (headers + PR references).

Line 7–13 uses non-standard headers and omits mandatory PR references in entries. Please switch to the required category names/emojis and include ([#PR](full-url) by [@contributor](profile-url)) for each item.

As per coding guidelines, "Use the following category headers with emojis: 🛠 Breaking changes, 🎉 New features, 🐛 Bug fixes, 📚 3rd party library updates, and 💡 Others" and "Each changelog entry must follow the format: Description... ([#PR](...) by [@contributor](...))". As per path instructions, "Ensure CHANGELOG.md updates follow the established format ... Each entry must include PR reference and author."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/react/src/components/PlanDisplay/CHANGELOG.md` around lines 7 - 13,
The CHANGELOG.md file uses non-standard headers and incomplete PR references
that don't match the repository contract. Update the category headers from "###
✨ Features" to "### 🎉 New features" and from "### ⚠️ Breaking changes" to "###
🛠 Breaking changes". Ensure each changelog entry follows the required format by
including the complete PR reference as `([`#PR`](full-url) by
[`@contributor`](profile-url))` at the end of each item description. The entries
mentioning the PlanDisplay rework and the highlighted prop removal need to have
their PR references properly formatted with the full GitHub URL and contributor
profile URL.

Sources: Coding guidelines, Path instructions

Comment on lines 14 to 19
as="span"
color={disabled ? "neutral-interactive" : "success-interactive"}
color={disabled ? "neutral-interactive" : "primary-interactive"}
>
<Box display="flex" alignItems="center">
{icon}
</Box>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Avoid nesting a default Box inside Text as="span".

On Line 17, Box is rendered inside Text as="span". If Box renders as a block element, this creates invalid HTML nesting and can cause inconsistent layout/semantics. Render that inner Box as a span (or remove the wrapper).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayBullet/PlanDisplayBullet.tsx`
around lines 14 - 19, The Box component is nested inside the Text component that
renders as a span (inline element), which creates invalid HTML when Box renders
as a block element. To fix this, modify the Box component to render as a span by
adding the as="span" prop, or if the Box wrapper is not necessary for layout
purposes, remove it entirely and place the icon directly inside the Text
component with appropriate flex styling applied to the Text element instead.

Comment on lines +28 to +33
<Box
as="span"
display="inline-flex"
gap="2"
style={{ verticalAlign: "middle" }}
>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Remove inline style usage in the badge wrapper.

Line 32 introduces a custom inline style (style={{ verticalAlign: "middle" }}), which violates repository styling rules. Please replace this with design-system-supported props/layout primitives.
As per coding guidelines **/*.{tsx,ts,scss,css}: Do NOT use custom styles in this repository; never use custom SCSS/CSS.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayBullet/PlanDisplayBullet.tsx`
around lines 28 - 33, Remove the inline style prop containing verticalAlign:
"middle" from the Box component wrapper in the PlanDisplayBullet component.
Replace it with design-system-supported alignment props instead. Since the
intent is vertical alignment, use the appropriate Box layout prop (such as
alignItems) that matches the design system's conventions for handling vertical
centering, removing the custom inline style entirely.

Source: Coding guidelines

Comment on lines 3 to +10
export type PlanDisplayBulletProperties = {
icon: ReactNode;
disabled?: boolean;
/**
* Optional slot rendered inline after the bullet text (e.g. a `<Tag>` like "Nuevo").
* @TJS-type React.ReactNode
*/
badge?: ReactNode;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use an interface for PlanDisplayBulletProperties in .types.ts.

PlanDisplayBulletProperties is currently declared as a type alias. Repository rules for *.types.ts require ComponentNameProperties to be an exported interface.

As per coding guidelines, "**/*.types.ts: Export a ComponentNameProperties interface following PascalCase convention..." As per path instructions, "**/*.types.ts: Always export a 'ComponentNameProperties' interface..."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayBullet/planDisplayBullet.types.ts`
around lines 3 - 10, Convert the PlanDisplayBulletProperties type alias to an
exported interface. Replace the type keyword and assignment syntax with
interface syntax, keeping all the properties (icon, disabled, badge) and their
JSDoc comments intact. This aligns with the repository's coding guidelines that
require ComponentNameProperties to be exported as an interface in *.types.ts
files.

Sources: Coding guidelines, Path instructions

Comment on lines +21 to +32
it("SHOULD render the gradient variant correctly", () => {
makeSut({ gradient: true });

expect(screen.getByText("Body content")).toBeDefined();
});

it("SHOULD render the ribbon label when provided", () => {
makeSut({ ribbonLabel: "Más escogido" });

expect(screen.getByText("Más escogido")).toBeDefined();
expect(screen.getByText("Body content")).toBeDefined();
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a precedence test for ribbonLabel over gradient.

The public contract documents that ribbonLabel overrides gradient, but this behavior is not directly tested. Add a case passing both props and asserting the ribbon path is rendered.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCard/planDisplayCard.spec.tsx`
around lines 21 - 32, Add a new test case in the planDisplayCard.spec.tsx file
that verifies the precedence behavior where ribbonLabel overrides gradient when
both props are provided. Create a test using makeSut that passes both gradient:
true and a ribbonLabel value, then assert that the ribbon label text is
rendered, confirming that the ribbon takes precedence over the gradient variant
as documented in the public contract.

Comment on lines +5 to +44
const PlanDisplayCardPrice: React.FC<PlanDisplayCardPriceProps> = ({
price,
previousPrice,
period,
annualNote,
}) => (
<Box display="flex" flexDirection="column" gap="2">
<Box display="flex" alignItems="center" gap="0-5">
<Title as="h3" color="neutral-textHigh">
{price}
</Title>
{previousPrice && (
<Text as="span" fontSize="base">
{/* `neutral-interactivePressed` is not exposed by Text's `color` prop,
so the token is applied via its CSS variable. */}
<span
style={{
color: "var(--nimbus-colors-neutral-interactivePressed)",
textDecoration: "line-through",
}}
>
{previousPrice}
</span>
</Text>
)}
{period && (
<Text as="span" color="neutral-textHigh">
{period}
</Text>
)}
</Box>
{annualNote && (
<Text as="p" fontSize="caption" color="neutral-textLow">
{annualNote}
</Text>
)}
</Box>
);

export { PlanDisplayCardPrice };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add component JSDoc and explicit displayName.

PlanDisplayCardPrice is missing component-level JSDoc and PlanDisplayCardPrice.displayName, both required by the TSX standards in this repo.

Proposed update
+/**
+ * Renders the pricing block for a plan card, including current price and optional
+ * previous price, billing period, and annual note.
+ */
 const PlanDisplayCardPrice: React.FC<PlanDisplayCardPriceProps> = ({
   price,
   previousPrice,
   period,
   annualNote,
 }) => (
   <Box display="flex" flexDirection="column" gap="2">
@@
   </Box>
 );
 
+PlanDisplayCardPrice.displayName = "PlanDisplayCardPrice";
+
 export { PlanDisplayCardPrice };

As per coding guidelines, "Set the displayName property for each component and sub-component in React" and "Document all React components with JSDoc comments including description and prop documentation."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const PlanDisplayCardPrice: React.FC<PlanDisplayCardPriceProps> = ({
price,
previousPrice,
period,
annualNote,
}) => (
<Box display="flex" flexDirection="column" gap="2">
<Box display="flex" alignItems="center" gap="0-5">
<Title as="h3" color="neutral-textHigh">
{price}
</Title>
{previousPrice && (
<Text as="span" fontSize="base">
{/* `neutral-interactivePressed` is not exposed by Text's `color` prop,
so the token is applied via its CSS variable. */}
<span
style={{
color: "var(--nimbus-colors-neutral-interactivePressed)",
textDecoration: "line-through",
}}
>
{previousPrice}
</span>
</Text>
)}
{period && (
<Text as="span" color="neutral-textHigh">
{period}
</Text>
)}
</Box>
{annualNote && (
<Text as="p" fontSize="caption" color="neutral-textLow">
{annualNote}
</Text>
)}
</Box>
);
export { PlanDisplayCardPrice };
/**
* Renders the pricing block for a plan card, including current price and optional
* previous price, billing period, and annual note.
*/
const PlanDisplayCardPrice: React.FC<PlanDisplayCardPriceProps> = ({
price,
previousPrice,
period,
annualNote,
}) => (
<Box display="flex" flexDirection="column" gap="2">
<Box display="flex" alignItems="center" gap="0-5">
<Title as="h3" color="neutral-textHigh">
{price}
</Title>
{previousPrice && (
<Text as="span" fontSize="base">
{/* `neutral-interactivePressed` is not exposed by Text's `color` prop,
so the token is applied via its CSS variable. */}
<span
style={{
color: "var(--nimbus-colors-neutral-interactivePressed)",
textDecoration: "line-through",
}}
>
{previousPrice}
</span>
</Text>
)}
{period && (
<Text as="span" color="neutral-textHigh">
{period}
</Text>
)}
</Box>
{annualNote && (
<Text as="p" fontSize="caption" color="neutral-textLow">
{annualNote}
</Text>
)}
</Box>
);
PlanDisplayCardPrice.displayName = "PlanDisplayCardPrice";
export { PlanDisplayCardPrice };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/PlanDisplayCardPrice.tsx`
around lines 5 - 44, The PlanDisplayCardPrice component is missing required
documentation and displayName property per the repo's TSX standards. Add a JSDoc
comment block above the component definition that includes a description of the
component's purpose and documents all props (price, previousPrice, period,
annualNote) with their types and descriptions. Additionally, add
PlanDisplayCardPrice.displayName = "PlanDisplayCardPrice" after the component
export to explicitly set the display name for debugging and React DevTools
identification.

Sources: Coding guidelines, Path instructions

Comment on lines +20 to +24
<span
style={{
color: "var(--nimbus-colors-neutral-interactivePressed)",
textDecoration: "line-through",
}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Remove inline custom styling from previousPrice rendering.

This introduces custom styling in a .tsx file (style={{ ... }}), which violates repository styling policy. Use Nimbus-supported props/tokens only (or expose the token through the design-system API) instead of inline CSS.

As per coding guidelines, "**/*.{tsx,ts,scss,css}: Do NOT use custom styles in this repository; never use custom SCSS/CSS."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/PlanDisplayCardPrice.tsx`
around lines 20 - 24, Remove the inline style object from the previousPrice span
element in PlanDisplayCardPrice.tsx that contains custom color and
textDecoration properties. Replace this inline styling with appropriate Nimbus
design-system tokens and props. Check if Nimbus provides component props or
token utilities for applying neutral-interactivePressed color and line-through
text decoration, and use those instead of the style={{ }} object.

Sources: Coding guidelines, Path instructions

Comment on lines +3 to +27
export type PlanDisplayCardPriceProperties = {
/**
* The current price of the plan (e.g. "$999"). Rendered as the main, prominent value.
* @TJS-type React.ReactNode
*/
price: ReactNode;
/**
* Optional previous price, rendered with a line-through next to the current price.
* @TJS-type React.ReactNode
*/
previousPrice?: ReactNode;
/**
* Optional billing period suffix (e.g. "/mes") rendered next to the price.
* @TJS-type React.ReactNode
*/
period?: ReactNode;
/**
* Optional annual note rendered below the price (e.g. "R$696/año. Economiza R$132.").
* Accepts a ReactNode so it can mix colors (e.g. an emphasized savings span).
* @TJS-type React.ReactNode
*/
annualNote?: ReactNode;
};

export type PlanDisplayCardPriceProps = PlanDisplayCardPriceProperties;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use an exported PlanDisplayCardPriceProperties interface (with top-level JSDoc).

In *.types.ts, this repository requires ComponentNameProperties to be exported as an interface. Here it is a type alias. Also add top-level JSDoc for exported types (not only field-level comments).

Proposed update
 import { ReactNode } from "react";
 
-export type PlanDisplayCardPriceProperties = {
+/**
+ * Properties contract for PlanDisplayCardPrice.
+ */
+export interface PlanDisplayCardPriceProperties {
@@
-};
+}
 
+/**
+ * Public props alias for PlanDisplayCardPrice.
+ */
 export type PlanDisplayCardPriceProps = PlanDisplayCardPriceProperties;

As per path instructions, "**/*.types.ts: Always export a 'ComponentNameProperties' interface ..." and "Use proper JSDoc comments for all exported types`."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export type PlanDisplayCardPriceProperties = {
/**
* The current price of the plan (e.g. "$999"). Rendered as the main, prominent value.
* @TJS-type React.ReactNode
*/
price: ReactNode;
/**
* Optional previous price, rendered with a line-through next to the current price.
* @TJS-type React.ReactNode
*/
previousPrice?: ReactNode;
/**
* Optional billing period suffix (e.g. "/mes") rendered next to the price.
* @TJS-type React.ReactNode
*/
period?: ReactNode;
/**
* Optional annual note rendered below the price (e.g. "R$696/año. Economiza R$132.").
* Accepts a ReactNode so it can mix colors (e.g. an emphasized savings span).
* @TJS-type React.ReactNode
*/
annualNote?: ReactNode;
};
export type PlanDisplayCardPriceProps = PlanDisplayCardPriceProperties;
import { ReactNode } from "react";
/**
* Properties contract for PlanDisplayCardPrice.
*/
export interface PlanDisplayCardPriceProperties {
/**
* The current price of the plan (e.g. "$999"). Rendered as the main, prominent value.
* `@TJS-type` React.ReactNode
*/
price: ReactNode;
/**
* Optional previous price, rendered with a line-through next to the current price.
* `@TJS-type` React.ReactNode
*/
previousPrice?: ReactNode;
/**
* Optional billing period suffix (e.g. "/mes") rendered next to the price.
* `@TJS-type` React.ReactNode
*/
period?: ReactNode;
/**
* Optional annual note rendered below the price (e.g. "R$696/año. Economiza R$132.").
* Accepts a ReactNode so it can mix colors (e.g. an emphasized savings span).
* `@TJS-type` React.ReactNode
*/
annualNote?: ReactNode;
}
/**
* Public props alias for PlanDisplayCardPrice.
*/
export type PlanDisplayCardPriceProps = PlanDisplayCardPriceProperties;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardPrice/planDisplayCardPrice.types.ts`
around lines 3 - 27, Convert the PlanDisplayCardPriceProperties type alias to an
interface declaration and add a top-level JSDoc comment above it. Change the
`export type PlanDisplayCardPriceProperties = { ... }` syntax to `export
interface PlanDisplayCardPriceProperties { ... }` and add a descriptive JSDoc
comment above the interface that explains what this type represents, keeping the
existing field-level JSDoc comments intact.

Sources: Coding guidelines, Path instructions

borderStyle="solid"
my="1"
/>
<Box {...props} my="3">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Let consumer spacing props override the default margin.

Line 8 applies my="3" after spreading props, which blocks callers from overriding margin. For better composability, set defaults before spreading props.

Suggested change
-  <Box {...props} my="3">
+  <Box my="3" {...props}>
     <Divider />
   </Box>

As per path instructions **/*.{ts,tsx}: review for component reusability and composability.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/react/src/components/PlanDisplay/src/components/PlanDisplayCardSpacing/PlanDisplayCardSpacing.tsx`
at line 8, The Box component in PlanDisplayCardSpacing.tsx currently has the
my="3" margin applied after the {...props} spread, which prevents consumers from
overriding the default margin value. Reorder the attributes so that the default
spacing (my="3") is set before spreading the props, allowing any margin-related
props passed by consumers to override the default. Change the order from
`{...props} my="3"` to `my="3" {...props}`.

Source: Path instructions

Comment on lines +155 to +184
type PlaygroundArgs = {
showTag: boolean;
showDiscount: boolean;
showSuffix: boolean;
showDescription: boolean;
showButton: boolean;
showFooter: boolean;
};

/**
* Mirrors the configurable properties of the Figma master component. Each toggle maps to an
* optional slot/prop, showing how every part of a plan card is opt-in through composition.
*/
export const Playground: StoryObj<PlaygroundArgs> = {
args: {
showTag: true,
showDiscount: true,
showSuffix: true,
showDescription: true,
showButton: true,
showFooter: true,
},
argTypes: {
showTag: { control: "boolean" },
showDiscount: { control: "boolean" },
showSuffix: { control: "boolean" },
showDescription: { control: "boolean" },
showButton: { control: "boolean" },
showFooter: { control: "boolean" },
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add controls for the newly exposed PlanDisplay props, not only visibility toggles.

Line 155–184 currently exposes boolean toggles only; key API inputs like ribbonLabel, gradient, price, previousPrice, period, and annualNote content aren’t controllable from Storybook controls, which weakens discoverability of the new surface.

As per coding guidelines, "**/*.stories.{ts,tsx}: Add controls for all component props in Storybook stories."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/react/src/components/PlanDisplay/src/planDisplay.stories.tsx` around
lines 155 - 184, The Playground story object only exposes boolean visibility
toggles through PlaygroundArgs type and argTypes, but does not include controls
for important content props like ribbonLabel, gradient, price, previousPrice,
period, and annualNote. Add these missing props to the PlaygroundArgs type
definition with appropriate types, provide default values for each in the args
object, and define corresponding controls in the argTypes object using
appropriate control types (text for string values, number for numeric values,
etc.) to make all component props discoverable and configurable in Storybook.

Source: Coding guidelines

@codecov-commenter

codecov-commenter commented Jun 23, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.38%. Comparing base (094b175) to head (28e879e).
⚠️ Report is 1 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #170      +/-   ##
==========================================
+ Coverage   93.25%   93.38%   +0.12%     
==========================================
  Files         223      225       +2     
  Lines        1749     1783      +34     
  Branches      359      380      +21     
==========================================
+ Hits         1631     1665      +34     
  Misses        113      113              
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

noecondoleo and others added 8 commits June 23, 2026 14:16
- Add TwoPlans, ThreePlans, Horizontal and HorizontalMobile stories plus Playground controls
- Increase bullet list gap to spacing 3 in card content
- Match subtitle color (neutral-textHigh) across header tag/no-tag branches
- HorizontalMobile uses a single spacing 3 between header and description (no divider), matching horizontal desktop

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Stretch cards to equal height with footers anchored at the bottom, and
reserve the ribbon's top space on every card so content rows align across
the grid. Remove the annual-price note from the TwoPlans and ThreePlans
stories.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ents

@nimbus-ds/components Box drops the `style` prop (it renders only
sprinkle-generated styles), so every `style={}` on a Box was a silent
no-op. This broke the Plans 2.0 gradient, the footer's `margin-top: auto`
bottom-alignment, the inline badge `vertical-align`, and the story layouts.

Route the gradient background, footer anchoring, badge alignment and
consumer `style` through native elements instead, matching the pattern
already used for the ribbon and struck-through price.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds back the disabled (X) bullets across PlanDisplay stories to show
locked features per plan tier, and centers the bullet icon with the
text line-height instead of top-aligning it.
yarn version check failed because the g:webpack quoting fix modified
this workspace without a release decision recorded.
@github-actions

Copy link
Copy Markdown
Contributor

🚀✨ Your Storybook preview is ready!

🔗 View Storybook

Happy reviewing! 🎉

@sonarqubecloud

Copy link
Copy Markdown

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the PlanDisplay Plans 2.0 rework after syncing this branch with master (merge commit d9931e5, no conflicts — origin/master hadn't touched anything under .cursor/rules/, .coderabbit.yaml, or PlanDisplay itself since this branch diverged, so no new construction rules landed, but I used the existing rules there — .cursor/rules/components.mdc, types.mdc, changelogs.mdc, json-docs.mdc, and .coderabbit.yaml's path_instructions/custom_checks — as the lens for this review). Lint, tsc --noEmit, and the full PlanDisplay Jest suite (9 suites / 16 tests) all pass on the merged branch.

Overall this is a solid, well-tested rework — the new PlanDisplay.Price subcomponent, the tag/badge/icon slots, and the ribbonLabel/gradient Card variants are sensibly composed, each has its own spec file, the breaking removal of highlighted is intentionally called out in code/README, and Storybook coverage is thorough. The PlanDisplay.Header tag slot change in particular is a clean example of extending a component through @nimbus-ds/components primitives with no native-element workarounds.

There are two issues I'd want fixed before merging (both flagged inline), which is why this is Request Changes rather than Comment:

  1. The breaking change is versioned as a minor bump, not major (.yarn/versions/plan-display-plans-2.yml). The removed highlighted prop is a real breaking change — the CHANGELOG entry in this same PR even titles it 2.0.0 — but the actual release-type declaration that yarn bump:stable will act on says minor, which would ship 1.2.0 instead. This is precisely what the repo's own custom_checks."Unversioned Breaking Changes" check is meant to catch.
  2. PlanDisplay.Price's generated docs are broken. planDisplay.docs.json was never regenerated in this PR (no diff despite Card/Bullet/Header prop changes), and — separately — the Price subcomponent's displayName doesn't follow the naming convention the doc generator relies on, so even after running yarn build:docs its entry comes out with 0 documented props. I verified both by running yarn build:docs locally on this branch (details + reproduction in the inline comments).

Additional smaller items (not blocking, in the inline comments or noted here):

  • The Card and Footer rewrites replace @nimbus-ds/components' Card/Box with native <div> + inline styles in several places. I verified the stated reason (Box truly discards the style prop at runtime) so this isn't unfounded, but it's still a deviation from "no native HTML elements / no custom CSS" that's worth a second look — see inline comments for specifics and a suggestion to file the Box style behavior as an upstream issue.
  • planDisplay.types.ts's PlanDisplayComponents.Card JSDoc (around the Card: property, outside this PR's diff hunks so I can't anchor a line comment on it) still says the Card "is able to receive a 'highlighted' prop to feature a plan" — stale now that the prop is removed.
  • This PR also carries an unrelated one-line fix to packages/core/webpack/package.json (quoting $INIT_CWD) with a patch bump for @nimbus-ds/patterns-webpack, but no corresponding CHANGELOG entry for that package. Minor scope creep — probably fine to keep bundled in, but it should have its own changelog line per .cursor/rules/changelogs.mdc.

Happy to re-review once the version bump and docs are fixed.


Generated by Claude Code

PlanDisplay.Card.displayName = "PlanDisplay.Card";
PlanDisplay.Bullet.displayName = "PlanDisplay.Bullet";
PlanDisplay.Header.displayName = "PlanDisplay.CardHeader";
PlanDisplay.Price.displayName = "PlanDisplay.Price";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: PlanDisplay.Price's generated docs come out empty because of this displayName.

Every other subcomponent here sets its public displayName to its internal component name so it matches the <Name>Properties type the docs generator (@nimbus-ds/scripts Docgen) looks up — e.g. PlanDisplay.Header.displayName = "PlanDisplay.CardHeader" resolves to PlanDisplayCardHeaderProperties. Price breaks that pattern: it's set to the public name "PlanDisplay.Price", so the generator looks for a type named PlanDisplayPriceProperties, but the actual exported type is PlanDisplayCardPriceProperties (in planDisplayCardPrice.types.ts).

I confirmed this by running yarn build:docs on this branch — it prints:

⚠️  Schema generation skipped for PlanDisplayPriceProperties: type PlanDisplayPriceProperties not found

and the resulting planDisplay.docs.json entry for PlanDisplay.Price comes out as {"name": "PlanDisplay.Price", "totalProps": 0, "props": []} — the flagship new subcomponent of this PR ships with zero documented props.

Also, planDisplay.docs.json itself was never regenerated in this PR at all (its diff against master is empty), even though props were added/removed/renamed across Card, Bullet, Header. Per .cursor/rules/json-docs.mdc this file must be regenerated via yarn build:docs after type changes — right now the published docs still show the removed highlighted prop and are missing gradient, ribbonLabel, tag, badge, icon, and PlanDisplay.Price entirely.

Fix: rename this to PlanDisplay.Price.displayName = "PlanDisplay.CardPrice"; (matching the type name), then run yarn build:docs and commit the regenerated planDisplay.docs.json.

@@ -0,0 +1,6 @@
releases:
"@nimbus-ds/plan-display": minor

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a major release, not minor. This PR removes the public highlighted prop from PlanDisplay.Card (see planDisplayCard.types.ts), which the CHANGELOG in this same PR correctly labels a "🛠/⚠️ Breaking change" and versions as 2.0.0. A minor release entry here means yarn bump:stable will actually bump @nimbus-ds/plan-display to 1.2.0, not 2.0.0 — contradicting the CHANGELOG and shipping a breaking API removal under a non-major version, which silently breaks consumers pinned with ^1.x.

This is also exactly the case the repo's own .coderabbit.yaml custom_checks.Unversioned Breaking Changes is designed to catch: "All breaking changes MUST have a MAJOR version bump (x.0.0)."

Please change this to:

releases:
  "@nimbus-ds/plan-display": major
  "@nimbus-ds/patterns-webpack": patch

Comment on lines +7 to +13
### ✨ Features

- Reworked `PlanDisplay` to the Plans 2.0 design as the canonical layout, matching the Figma master component. `PlanDisplay.Card` now renders the default card with a level-2 shadow, plus two opt-in variants: `ribbonLabel` (a "Más escogido" ribbon with a primary 2px border) and `gradient` (primary-surface → background gradient). Added a `tag` slot on `PlanDisplay.Header` (e.g. "Plan actual"), a new `PlanDisplay.Price` subcomponent (current price + optional previous price, period and annual note), a `badge` slot on `PlanDisplay.Bullet` (e.g. "Nuevo"), and an `icon` slot on `PlanDisplay.Footer` for add-on rows. (by [@noecondoleo](https://github.com/noecondoleo))

### ⚠️ Breaking changes

- Removed the legacy `highlighted` prop on `PlanDisplay.Card`. Use `ribbonLabel` for the featured plan or `gradient` for the gradient variant. (by [@noecondoleo](https://github.com/noecondoleo))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting deviates from .cursor/rules/changelogs.mdc in two ways that matter beyond nitpicking, because the second one is also literally what the repo's custom_checks."Breaking Change Documentation Verification" cross-references for:

  1. Category headers should be #### (not ###) with the exact emojis 🎉 New features and 🛠 Breaking changes (not ✨ Features / ⚠️ Breaking changes). The automated check looks for the literal string "🛠 Breaking changes" — as written, this section won't match it.
  2. Entries are missing the PR reference link. The rule's required format is - Description. ([#PR](url) by [@contributor](url)); both bullets here only have the contributor link, no [#170](https://github.com/TiendaNube/nimbus-patterns/pull/170).

Suggested fix:

#### 🎉 New features

- Reworked `PlanDisplay` to the Plans 2.0 design... ([#170](https://github.com/TiendaNube/nimbus-patterns/pull/170) by [@noecondoleo](https://github.com/noecondoleo))

#### 🛠 Breaking changes

- Removed the legacy `highlighted` prop on `PlanDisplay.Card`... ([#170](https://github.com/TiendaNube/nimbus-patterns/pull/170) by [@noecondoleo](https://github.com/noecondoleo))

Comment on lines +18 to 76
<Box {...rest} display="flex" flexDirection="column" height="100%">
{/* Ribbon area: visible when `ribbonLabel` is set, an invisible placeholder of the
same height otherwise. Rendering it on every card reserves identical top space so
the content rows (subtitle, price, bullets) align across the grid. */}
<div
aria-hidden={hasRibbon ? undefined : true}
style={{
display: "flex",
justifyContent: "center",
alignItems: "center",
backgroundColor: hasRibbon
? "var(--nimbus-colors-primary-interactive)"
: "transparent",
paddingTop: "var(--nimbus-spacing-0-5)",
paddingBottom: "var(--nimbus-spacing-2)",
borderTopLeftRadius: "var(--nimbus-shape-border-radius-2)",
borderTopRightRadius: "var(--nimbus-shape-border-radius-2)",
marginBottom: "calc(var(--nimbus-spacing-2) * -1)",
}}
>
<Text fontWeight="medium" color="neutral-background" textAlign="center">
{ribbonLabel || " "}
</Text>
</div>
{/* Content grows to fill the card height (cards in a row stretch to match), so footers
anchored with `margin-top: auto` line up at the bottom. */}
<Box
flex="1 1 auto"
display="flex"
flexDirection="column"
backgroundColor="neutral-background"
borderRadius="2"
overflow="hidden"
{...(hasRibbon
? {
borderColor: "primary-interactive",
borderStyle: "solid",
borderWidth: "2",
}
: { boxShadow: "2" })}
>
{/* Content lives in a native element because @nimbus-ds/components Box drops the
`style` prop (it renders only sprinkle-generated styles). Routing the gradient
background and any consumer `style` through here is the only way they take effect.
The div fills the card as a flex column so footers still anchor with `margin-top: auto`. */}
<div
style={{
display: "flex",
flexDirection: "column",
flex: "1 1 auto",
padding: "var(--nimbus-spacing-4)",
background: gradient && !hasRibbon ? GRADIENT_BACKGROUND : undefined,
...style,
}}
>
{children}
</div>
</Box>
</Box>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This drops the Card import from @nimbus-ds/components entirely and rebuilds the card chrome (shadow, border, radius, overflow, ribbon, gradient) with two native <div>s and hand-written inline styles / raw CSS variables. That's a direct conflict with two explicit project rules: .cursor/rules/components.mdc ("We do NOT use custom styles in this repository. NEVER use custom SCSS/CSS") and .coderabbit.yaml's review instructions ("Consistent usage of @nimbus-ds/components, instead of native HTML elements").

I verified the stated justification in the comment on lines 59-62 — @nimbus-ds/components' Box really does silently discard the style prop at runtime (its dist/Box/index.js destructures style from props and then never uses it, using only the sprinkle-generated style instead) — so falling back to a native element for margin-top: auto / a custom gradient is a real, verified constraint, not an invented one. Given that, this is more of a design-system limitation than a bug in this PR.

That said, worth reconsidering before merging:

  • Several of the values hardcoded here as raw CSS vars (backgroundColor, paddingTop/paddingBottom, borderTopLeftRadius/borderTopRightRadius) are expressible through Box's own token props (backgroundColor, pt/pb) — only the per-corner radius and the gradient/margin-top:auto seem to genuinely need the escape hatch. Narrowing the native-element usage to just what's unavoidable would reduce the surface that's now unstyleable via the design system's normal props.
  • PlanDisplayCardContent (unchanged in this PR) still renders <Card.Body> as a child of this now-Card-less wrapper. Card.Body doesn't require a <Card> ancestor to render correctly (confirmed — it's a self-contained padded box), but it's a bit of an inconsistent signal to keep the DS's Card.Body while replacing its sibling <Card> with hand-rolled divs.
  • Since Box silently ignoring style is a real footgun (it's still typed as accepted, so this compiles fine while doing nothing), it might be worth filing that as an issue against @nimbus-ds/components — other component authors are likely to hit this same silent no-op.

Comment on lines +12 to +29
<div style={{ display: "flex", flexDirection: "column", marginTop: "auto" }}>
<Box mt="3" mb="3">
<Divider />
</Box>

<Box>{children}</Box>
</Box>
{icon ? (
<Box display="flex" gap="2" alignItems="flex-start">
<Text as="span" color="primary-interactive">
<Box display="flex" alignItems="center">
{icon}
</Box>
</Text>
<Box>{children}</Box>
</Box>
) : (
<Box>{children}</Box>
)}
</div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same native-<div>-for-style pattern as PlanDisplayCard (justified — Box does drop style, see the other comment on PlanDisplayCard.tsx), but this one also drops the existing <PlanDisplayCardSpacing /> subcomponent in favor of re-implementing the same divider inline (<Box mt="3" mb="3"><Divider /></Box> here vs. PlanDisplayCardSpacing, which after this PR is literally <Box my="3"><Divider /></Box>). Minor, but it's now duplicated in two places with slightly different spacing props (mt+mb vs my) for what should be the same visual divider — worth reusing <PlanDisplayCardSpacing /> here instead, both for DRYness and so a future spacing change doesn't need to be made in two files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants